Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

WIP: Add psycopg3 as a new backend #586

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open

WIP: Add psycopg3 as a new backend #586

wants to merge 15 commits into from

Conversation

ansipunk
Copy link

@ansipunk ansipunk commented Mar 2, 2024 •
edited
Loading

Resolves #535.

I generally don't have time for this during weekdays, so if anyone wants to help out - feel free to fork my fork, we will merge it and make this thing work. Maybe @tomchristie still remembers how did he make it work for other drivers.

https://www.psycopg.org/psycopg3/docs/basic/adapt.html

TODO:

There are no timeouts specified for the driver, which in some cases might hang the process. Adding timeouts is fairly easy.

Currently there is a hacky solution to properly serialize and deserialize JSONs. While this works, this is far from perfect and we should figure out a better way to do it. Moreover, this hack will break compatibility with PostgreSQL's native Array type.

connection.adapters.register_loader("json", JsonLoader)
connection.adapters.register_loader("jsonb", JsonLoader)
connection.adapters.register_dumper(dict, JsonDumper)
connection.adapters.register_dumper(list, JsonDumper)

Possibly we can add orjson to optional dependencies to make serialization/deserialization faster.

And maybe remove aiopg entirely?

Not working:

  • Enums, Datetimes
  • Custom fields
  • Duplicate names in SELECT clauses
  • Returning values from cursor.execute() - while it's possible, it might throw an error sometimes due to empty results list. Might add a simple try/catch there later
  • Test with temporary tables

Done:

Asynchronous connection and connection pool using psycopg3.

Renamed postgres backend to asyncpg backend. Updated databases/core.py to use new backends and support new DSN schemas.

Mentioned that psycopg3 may be used in README and docs. Added the new driver as an optional dependency to setup.py.

Also updated tests that involve Starlette to work with their new API, otherwise the thing was throwing bunch of warnings.

Edited pipeline to use modern MariaDB instead of old MySQL - for some reason old MySQL image would kill performance on my machine and replacing it did the trick. I imagine this might be good for the pipelines, too, but needs some testing if necessary. Updated contributing page in the same fashion.

Test case status:

========================= short test summary info ==========================
FAILED tests/test_databases.py::test_results_support_column_reference[postgresql+psycopg://username:password@localhost:5432/testsuite]
FAILED tests/test_databases.py::test_result_values_allow_duplicate_names[postgresql+psycopg://username:password@localhost:5432/testsuite]
FAILED tests/test_databases.py::test_execute_return_val[postgresql+psycopg://username:password@localhost:5432/testsuite]
FAILED tests/test_databases.py::test_enum_field[postgresql+psycopg://username:password@localhost:5432/testsuite]
FAILED tests/test_databases.py::test_custom_field[postgresql+psycopg://username:password@localhost:5432/testsuite]
FAILED tests/test_databases.py::test_iterate_outside_transaction_with_temp_table[postgresql+psycopg://username:password@localhost:5432/testsuite]
================= 6 failed, 52 passed, 2 skipped in 9.86s ==================

@ansipunk ansipunk changed the title Add and test psycopg3 support WIP: Add and test psycopg3 support Mar 2, 2024
@ansipunk
Copy link
Author

ansipunk commented Mar 2, 2024

It's mostly ready, but I will double-check if I have missed anything, if there is any code that should have been updated.

@ansipunk
Copy link
Author

ansipunk commented Mar 2, 2024 •
edited
Loading

Never mind, this is not ready, there is a lot of work to do. backends/postgresql is hardcoded to use asyncpg no matter the specified dialect, so all the tests were actually working with it.

@tarsil
Copy link
Contributor

tarsil commented Mar 2, 2024

@ansipunk psycopg3 is called simply psycopg.

https://pypi.org/project/psycopg/

This version is already being supported.

@ansipunk
Copy link
Author

ansipunk commented Mar 2, 2024 •
edited
Loading

@tarsil Read databases/core.py and databases/backends/postresql.py please.

@tarsil
Copy link
Contributor

tarsil commented Mar 2, 2024

@ansipunk got it. We are talking 2 completely different things. You meant the scheme support.

Why is that needed? Normally postgres or asyncpg would suffice as internally is converted.

What is the use case?

@ansipunk
Copy link
Author

ansipunk commented Mar 2, 2024

@tarsil What? No. I mean conneciton management and connection pools. If you read the source code, you can see that it exclusively uses asyncpg or aiopg. If you could explain to me, how does that mean psycopg support, you might save me few hours of pointless work.

@ansipunk
Copy link
Author

ansipunk commented Mar 2, 2024

The use case is to use psycopg instead of asyncpg. With asyncpg we have to have psycopg anyway for SQLAlchemy-Utils and alembic to function. With this working we can remove the asyncpg dependency from code whatsoever.

@tarsil
Copy link
Contributor

tarsil commented Mar 2, 2024

@ansipunk ok, let's start over. Apologies if I'm lost here. Internally uses asyncpg and aiopg yes for async driver but here https://github.com/encode/databases/blob/master/databases/backends/postgres.py it uses the SQL alchemy 2 dialect for it and internally the psycopg being used is the one shared before here.

Now, based on your example we indeed do not support like that but it can be analysed and see what we could do to make it happen and if it's worth it.

I hope I'm explaining properly? Also SQLAlchemy utils until recently, if I'm not mistaken, was incompatible with SQLA 2. I think that was addressed recently but I'm not 100% sure, I need to investigate

@tarsil
Copy link
Contributor

tarsil commented Mar 2, 2024

The use case is to use psycopg instead of asyncpg. With asyncpg we have to have psycopg anyway for SQLAlchemy-Utils and alembic to function. With this working we can remove the asyncpg dependency from code whatsoever.

Let me do some investigation here and come back to you soon? I read about the async support with psycopg (now 3) as well but I'm not 100% yet familiar with what we can actually do.

@ansipunk
Copy link
Author

ansipunk commented Mar 3, 2024

As for the commit history, this is fucked up, hopefully we will just squash commits.

@ansipunk ansipunk force-pushed the add-psycopg3-support branch 2 times, most recently from bdb6dbe to ca7186a Compare March 3, 2024 10:38
@ansipunk ansipunk changed the title WIP: Add and test psycopg3 support WIP: Add psycopg3 as a new backend Mar 3, 2024
@ansipunk ansipunk force-pushed the add-psycopg3-support branch 2 times, most recently from f39f5eb to e7358af Compare March 5, 2024 19:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support for psycopg3?
2 participants
@ansipunk @tarsil